Skip to content

fix: lone high surrogate decoded as U+0000 instead of an error - #482

Merged
Licenser merged 2 commits into
simd-lite:mainfrom
leocaolab:fix/lone-high-surrogate
Oct 25, 2026
Merged

Licenser merged 2 commits into
simd-lite:mainfrom
leocaolab:fix/lone-high-surrogate

Conversation

@ddxd

@ddxd ddxd commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Fixes #481.

get_unicode_codepoint returned Ok((0, src_offset)) for a high surrogate not followed by a \u escape, and for a surrogate pair with invalid hex digits in either half. The 0 was the old "bytes written = 0 means failure" sentinel from handle_unicode_codepoint. Since 29d9998 the function returns a code point, so it now means U+0000 and "\ud800" parsed as "\0".

This change returns Err(ErrorType::InvalidUnicodeCodepoint) in both places. Every parse_str implementation already maps an error from here to InvalidUnicodeCodepoint at the escape's position, so no caller changes are needed.

Test: lone_high_surrogate_is_an_error covers "\ud800", "\ud800x", ["\udbff"], a surrogate inside a string, "\ud800\n", "\ud800\uzzzz", "\ud800A" and "\udc00" through both to_tape and to_borrowed_value, and checks that a valid pair still decodes. It fails without the fix (accepted: "\ud800") and passes with it.

cargo test passes on aarch64 (macOS, NEON) and x86_64 (Linux, AVX2), and cargo fmt --check is clean.

Side note, not changed here: the if o == 0 checks after handle_unicode_codepoint in the avx2/sse42/neon/simd128/portable parse_str can no longer trigger, because codepoint_to_utf8 writes at least one byte. I left them in to keep this diff minimal. Happy to remove them in a follow-up if you prefer.

🤖 Generated with Claude Code

Comment thread src/tests.rs Outdated
@symbiogenesis

symbiogenesis commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

At 0e253fb0, "\ud800\ue000" parses as U+10400 through the Serde, tape and borrowed-value APIs. The second escape is outside the low-surrogate range; serde_json rejects it.

In get_unicode_codepoint, check the full range before combining the pair:

if !LOW_SURROGATES.contains(&code_point_2) {
    return Err(ErrorType::InvalidUnicodeCodepoint);
}

checked_sub(0xdc00) checks only the lower bound and accepts values above 0xdfff. A 12-case Windows ARM64 probe agrees with serde_json 1.0.154 after this range check, including valid boundary pairs, NUL and emoji. Codex-assisted verification.

get_unicode_codepoint returned Ok((0, _)) for a high surrogate that is
not followed by a `\u` escape, and for a surrogate pair with invalid hex
digits. The 0 is a leftover sentinel: when this code returned the number
of bytes written, 0 meant failure and callers checked `o == 0`. Since
29d9998 it returns the code point instead, so the 0 became U+0000 and
"\ud800" parsed as "\0" without an error.

Return InvalidUnicodeCodepoint in both places, and add a regression test
covering the lone-surrogate forms (fails before this change).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Licenser
Licenser force-pushed the fix/lone-high-surrogate branch from 0e253fb to 6121944 Compare October 25, 2026 13:03
@ddxd

ddxd commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks @symbiogenesis for the report — confirmed: "\ud800\ue000" (and \ud800\uffff, \udbff\ue000, \udbff\uffff) decoded as a code point above U+FFFF instead of an error, because only the lower bound of the second escape was checked.

af7f553 checks the second escape against LOW_SURROGATES before combining the pair, which also covers invalid hex digits (they set the high bits), so the separate >> 16 check and the two checked_subs are gone. The test now covers both bounds of the low-surrogate range, the serde path (behind serde_impl), and the first and last valid pairs (U+10000, U+10FFFF); the new cases fail without the fix. cargo test --lib passes with default and --no-default-features; clippy clean.

get_unicode_codepoint checked the second escape of a pair only against
the lower bound (checked_sub(0xdc00)), so "\ud800\ue000" decoded as
U+10400 where serde_json rejects it. Check the second escape against
LOW_SURROGATES before combining; invalid hex digits set the high bits
and fail the same check.

The test now covers both bounds of the low-surrogate range, the serde
path, and the first and last valid pairs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ddxd
ddxd force-pushed the fix/lone-high-surrogate branch from 7348eb5 to af7f553 Compare October 25, 2026 13:37
@Licenser
Licenser merged commit ff8f5c8 into simd-lite:main Oct 25, 2026
@Licenser

Copy link
Copy Markdown
Member

thanks @ddxd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lone high surrogate "\ud800" is decoded as U+0000 instead of an error

3 participants